-
Notifications
You must be signed in to change notification settings - Fork 29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix detection of error messages. #843
Conversation
This will make recovery of the "No registered event listener" error during local access of a Somfy box actually work. |
Thanks @shypike, we will have a look! Interesting that they throw a slight different error locally... How are you using the local integration? Via your own Python code or via HA? |
I've made a crude hack in Home Assistant where I have a custom component that monkey-patches some methods of the OverkizClient class of pyoverkiz. |
What I've seen so far, is that scenarios defined in Tahomalink don't work locally. So I had to redefine my scenarios in Home Assistant. |
@shypike we actually have a version of Overkiz with a local integration ready, for months, but it is not polished. If you are want to contribute, you are more than welcome. home-assistant/core#71644 See https://community.home-assistant.io/t/overkiz-integration-local-api-development-testers-topic/485264 |
@iMicknl I'll have a look, but with summer coming :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shypike we would prefer to only have the messages changes where there is no exact match, for clarity. Or even add exact string matches in the if statement, instead of an in message
.
Would you be able to make this change, so we can merge it in?
@iMicknl What is not clear to me is whether the "No registered event listener" is unique for local access. If it's valid for both local and remote, using exact string matches will not work. Unless your original code was already incorrect for remote. |
I will try to simulate the error on cloud API to see if this is incorrect in the client library, or if this is something that is using another error message in the local API. The reason we would like exact matches is too be as strict as possible, if Overkiz/Somfy would change the error messages, we probably would need changes anyway, thus it is good to have it fail. We rather have everything as strict as possible. Great to hear that your local hack is working great! We still need to full implement this in HA. Can you perhaps verify if you have the same issue? Somfy-Developer/Somfy-TaHoma-Developer-Mode#102 |
@iMicknl I have mostly RTS devices and only two IO screens. The RTS devices don't give any feedback, so always have state unknown. I'll keep an eye on IO. However, I've tried to set debug mode on but nothing is logged.
The above lines in the config should do the trick. What am I doing wrong? |
Don't try to do exact matches, but use "in". One issue being that the message usually looks like: '"No registered event listener."' (Note the extra quotes.)
dd43062
to
20f9ac2
Compare
My last commit just strips off outer single quotes. That should care of the difference between local and remote messages. |
Pfff, I stripped the wrong kind of quotes. When I read my first text again, I see that "local" adds double quotes and not single quotes.
|
@shypike can you save some JSON files of the error? So we can add testcases for this. |
Don't try to do exact matches, but use "in".
One issue being that the message usually looks like:
'"No registered event listener."'
(Note the extra quotes.)